ADBM-3574: Fix JSON support for pgbackrest recovery filters by VoidZeroNull0 · Pull Request #113 · arenadata/pgbackrest

VoidZeroNull0 · 2026-10-28T12:36:25Z

What happened?
Restore failed when a --filter value contained a "\uXXXX" escape
outside the ASCII range - e.g. a surrogate pair like "\uD83E\uDD52".
The parser only supported "\u00XX" and threw on anything else.

How do we fix this?
Cherry-pick relevant commits from the upstream, which fix the issue.
Add a test for jsonValidate() to check exactly for surrogate pairs.

Tests?
One success test for the surrogate-pair was added, else are comming
from cherry-picked commits.
Ticket: ADBM-3574

!!!REBASE ONLY!!!

VoidZeroNull0 · 2026-10-29T14:21:47Z

Tests seem to fail because of missing docker image. Nothing related to the patch.

KnightMurloc · 2026-10-30T07:59:14Z

Do we really need to actually decode UTF-16 strings? During WAL filtering, we do not use text table names.

jsonReadSkipStr() stepped past the terminating null when a backslash was the last character. It also advanced one character too far after a \uXXXX escape, which skipped the closing quote when the escape was the last character of the string and stepped past the terminating null when the escape was at the end of the input. jsonReadStr() did not check that both digits of a \u00XX escape were present, so a truncated escape at the end of the input was read past the terminating null. The skip path validates the JSON values in info files and manifests. The read path reads object store responses. Changes to the original commit: None. (cherry picked from commit b623071) Ticket: ADBM-3574

jsonReadStr() decoded a \u0000 escape and appended a null character to the string. This asserted in debug builds and created a string with an embedded null in production builds, which is cut short anywhere the string is used as a zero-terminated string. Error on \u0000. A string cannot store a null character. Changes to the original commit: None. (cherry picked from commit 1808295) Ticket: ADBM-3574

jsonReadSkipStr() required four decimal digits after \u, so a valid escape with hex digits such as \u003c failed validation even though jsonReadStr() decodes it. Accept hex digits so skipping agrees with reading. pgBackRest does not write \u escapes, so this only affects JSON from other sources. jsonReadStr() still decodes only \u00XX, as a single byte. Changes to the original commit: None. (cherry picked from commit b63357e) Ticket: ADBM-3574

VoidZeroNull0 · 2026-11-02T04:51:59Z

\u0000 results in a NUL byte in the string (while upstream now rejects this).

Now using patches from upstream. Should be ok.

VoidZeroNull0 · 2026-11-02T04:53:07Z

Do we really need to actually decode UTF-16 strings? During WAL filtering, we do not use text table names.

Good question! I agree that they are kind of useless, but I think we better follow the upstream way - fix deconding for table names as they did.

Add test for jsonValidate() containing surrogate pair (e.g. \uD83E\uDD52). Ticket: ADBM-3574

VoidZeroNull0 · 2026-11-02T05:46:02Z

I guess, adding test for jsonValidate() for surrogate pairs is enough, no need to up as partialRestoreTest.c. Added in b372783.

RekGRpth · 2026-11-02T06:22:46Z

!!!REBASE ONLY!!!

If we use a rebase merge, could we make the description of the final commit more detailed?

RekGRpth · 2026-11-02T06:23:55Z

Are you going to merge with failing tests?

VoidZeroNull0 marked this pull request as ready for review October 29, 2026 14:19